fix: Replace asyncio.iscoroutinefunction with compatibility shim for Python 3.16 #4915
Conversation
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
sentry_sdk/_compat.py
Outdated
| iscoroutinefunction = asyncio.iscoroutinefunction # type: ignore[assignment] | ||
|
|
||
| def markcoroutinefunction(func): | ||
| # type: (_F) -> _F | ||
| func._is_coroutine = asyncio.coroutines._is_coroutine # type: ignore | ||
| return func |
There was a problem hiding this comment.
Potential bug: The fallback for markcoroutinefunction uses asyncio.coroutines._is_coroutine, which was removed in Python 3.11, causing an AttributeError.
-
Description: The compatibility shim for
markcoroutinefunctionprovides a fallback for Python versions older than 3.12. This fallback incorrectly assumesasyncio.coroutines._is_coroutineexists. However, this attribute was removed in Python 3.11. When the SDK is used with a Django ASGI application on Python 3.11, the fallback logic is triggered, leading to anAttributeErrorwhenmarkcoroutinefunctionis called insentry_sdk/integrations/django/asgi.py. -
Suggested fix: The fallback implementation for
markcoroutinefunctionneeds to be updated to not useasyncio.coroutines._is_coroutine. A different implementation that is compatible with Python versions prior to 3.12, including 3.11, should be used.
severity: 0.85, confidence: 0.95
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Thanks for the PR @Dibbu-cell.
I have requested some initial changes.
I had a look at other libraries and there are differences at how they detect coroutines across different Python versions. I will take a another look at the changes in sentry_sdk.
We would be in line with asgiref with your changes: https://github.com/django/asgiref/blob/f587b122af17bdba5749c30b96d2237bc1c2dfdf/asgiref/sync.py#L61-L69
Starlette does a simpler version check.
https://github.com/Kludex/starlette/blob/3912d6313730cc6004dfb4436e37dbc1a81db7c8/starlette/_utils.py#L11-L15
It's worth investigating if there could be any differences with the methods for detecting full inspect and asyncio compatibility for different Python version.
sentry_sdk/ai/monitoring.py
Outdated
| if inspect.iscoroutinefunction(f): | ||
| if iscoroutinefunction(f): |
There was a problem hiding this comment.
In places we're already using inspect.iscoroutinefunction(f) it's best to avoid change. Can you revert the changes in this file?
| if asyncio.iscoroutinefunction(httpx_client.get): | ||
| if iscoroutinefunction(httpx_client.get): |
There was a problem hiding this comment.
We should be able to call inspect.iscoroutinefunction() directly here, since we control what runs in the tests.
| except Exception: | ||
| pass | ||
|
|
||
| if inspect.iscoroutinefunction(f): |
There was a problem hiding this comment.
Same here, no need for change where we already use inspect.iscoroutinefunction().
| if inspect.iscoroutinefunction(tool): | ||
| if iscoroutinefunction(tool): |
There was a problem hiding this comment.
The change here should also be reverted.
|
ok, i will do that. |
2 participants
Description
This PR fixes the deprecation warning for asyncio.iscoroutinefunction that will be removed in Python 3.16. The function was deprecated in Python 3.12 in favor of inspect.iscoroutinefunction.
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)